- 
                Notifications
    
You must be signed in to change notification settings  - Fork 30
 
[Enhancement|HiFi]Implement RTCAudioStore as Store #977
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Enhancement|HiFi]Implement RTCAudioStore as Store #977
Conversation
          
 Generated by 🚫 Danger  | 
    
85513ae    to
    e2789fd      
    Compare
  
    6d4ad42    to
    7ae8feb      
    Compare
  
    e2789fd    to
    855a425      
    Compare
  
    d27ed23    to
    d80c8a6      
    Compare
  
    855a425    to
    1f26c8c      
    Compare
  
    9f33fc8    to
    31d363b      
    Compare
  
    31d363b    to
    c5130b0      
    Compare
  
    eb2eb3b    to
    283a24a      
    Compare
  
    c5130b0    to
    b5b810e      
    Compare
  
            
          
                Sources/StreamVideo/Utils/AudioSession/AudioDeviceModule/AudioDeviceModule.swift
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                Sources/StreamVideo/Utils/AudioSession/AudioDeviceModule/AudioDeviceModule.swift
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                Sources/StreamVideo/Utils/AudioSession/AudioDeviceModule/AudioDeviceModule.swift
          
            Show resolved
            Hide resolved
        
              
          
                Sources/StreamVideo/Utils/AudioSession/AudioDeviceModule/AudioDeviceModule.swift
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                Sources/StreamVideo/Utils/AudioSession/AudioDeviceModule/AudioEngineLevelNodeAdapter.swift
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                Sources/StreamVideo/Utils/AudioSession/AudioDeviceModule/AudioEngineLevelNodeAdapter.swift
          
            Show resolved
            Hide resolved
        
      | } else if let audioRecorder = try? AVAudioRecorder.build() { | ||
| mode = .audioRecorder(audioRecorder) | ||
| } else { | ||
| mode = .invalid | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how can we end up here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the audioRecorder fail to get created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that's clear, I meant how can that happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AVAudioRecorder initializer is failable, so if for some reason that fails, we can end up there. It seems that the main reasons for it to fail is to pass an invalid configuration (we are using the same configuration since forever)
| 
               | 
          ||
| /// Abstraction over the WebRTC audio session that lets the store coordinate | ||
| /// audio behaviour without tying tests to the concrete implementation. | ||
| protocol AudioSessionProtocol: AnyObject { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we're abstracting too much of WebRTC? Might be harder to keep up with newer versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WebRTC attempts to make as minimum changes to platform specific as possible (especially on Audio for iOS). This abstraction is mostly abstracting things common to AVAudioSession and RTCAudioSession. It won't pose a problem when updating for sure.
          
 | 
    
        
          
                Sources/StreamVideo/Utils/AudioSession/AudioDeviceModule/AudioEngineLevelNodeAdapter.swift
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...a/xcbaselines/84F737F3287C13AD00A363F4.xcbaseline/405B1570-F1EC-4638-9C96-FA56CB97DF5B.plist
              
                Outdated
          
            Show resolved
            Hide resolved
        
      5fc3b4c    to
    a4cd163      
    Compare
  
    
          SDK Size
  | 
    
          Public Interface🚀 No changes affecting the public interface.  | 
    
          StreamVideo XCSize
 Show 88 more objects
  | 
    
          StreamVideoSwiftUI XCSize
  | 
    
ad2ecd8
      into
      
  
    enhancement/hifi/feature-implementation
  
    


Depends on #976
☑️ Contributor Checklist